Skip to content

fix: bound the six remaining retained-state leaks (#820 wave 2) - #892

Merged
lidge-jun merged 93 commits into
devfrom
codex/wt2-zero-leak-impl
Aug 3, 2026
Merged

fix: bound the six remaining retained-state leaks (#820 wave 2)#892
lidge-jun merged 93 commits into
devfrom
codex/wt2-zero-leak-impl

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Summary

Unified bug-fix campaign: the wt2 retained-state hardening plus all four planned workstreams (wt1 update path, wt4 server/config, wt5 Windows service, wt3 provider wire), reviewed per workstream by independent adversarial reviewers and verified end-to-end. Supersedes #840 #841 #843 #844 #845 #847 (closed earlier) and #871 #886 #887 #889 (closed as each workstream landed here).

wt2 — retained-state bounds (tracker #820 wave 2)

wt1 — update path

wt4 — server/config

wt5 — Windows service

wt3 — provider wire

wp6 — contributor bug-fix stack (5 PRs stacked, 2 triaged closed)

Validation

  • bun run test: 7,452 pass, 8 skip, 0 fail locally (full suite on daf7069, 499 files); every workstream's focused suites re-run after each stack
  • CI: all 14 checks green on 6ec6ffc — ubuntu / macos / windows full matrix, npm-global ×3, systemd/launchd/schtasks, react-doctor, enforce-target, CodeRabbit. Windows is green on this branch for the first time in the campaign (the dev-inherited matrix breakage is not present here)
  • CI stabilization folded during wp6 (test-only, runner-flake class): [Bug]: doctor repeats OPENCODEX_BUN_PATH guidance when a Bun canary override is already active #848 provenance test timeout headroom (a8e0fe0), dead-pid probe for the symlinked sweep (0bccc8a), STORE_BUDGET_MS for the two cleanup-mode tests (80c49cf), thrown-CIM-deadline tolerance in the live enumeration test (6ec6ffc)
  • bun x tsc --noEmit: pass
  • bun run privacy:scan: pass
  • Red-green verified per fix; 15 adversarial reviewer rounds across 6 independent reviewers, all findings folded or rebutted
  • Pre-push hook (typecheck + gui lint + full test + privacy scan + doctor:gui): pass on every push through eeef7a3; later heads were pushed with the hook skipped per maintainer instruction, with the identical full suite + typecheck + privacy:scan run manually first

Notes

  • No RSS/benchmark superiority claims (comparator cells remain UNKNOWN).
  • Co-developed with a second session on this branch (wt2 augmentations, wt1 commits, review-fix iterations); original PRs' analyses drove each fix.

Summary by CodeRabbit

  • New Features

    • Added ocx config commands for viewing, editing, validating, importing, and exporting configuration.
    • Added Codex prompt-layer management, including toggles, custom instructions, adoption, recovery, and safe rollback.
    • Added Bun runtime provenance to service diagnostics and launchers.
    • Expanded provider support for Copilot routing, Claude 1M context, service-tier capabilities, and reasoning replay.
  • Bug Fixes

    • Improved update detection, GitHub star-prompt deferral, CORS extension handling, symlink-safe writes, Windows service settling, and memory safeguards.
    • Improved error reporting for upstream translation-limit failures.

…d mechanisms

wp-a: modelWireDefaults on the github-copilot registry entry (mechanism
already on tree: hard pin > modelAdapters > registry defaults > provider
adapter); conservative 6-model set, nano/sol lead-only via modelAdapters.
wp-b: adopt PR #860's capability file map + #875 root cause found —
sanitizeReasoningInputContent blanks plaintext reasoning for every
Responses provider (openai-responses.ts:35, called :1027); scope it.
wp-c: registry.ts:217 map + authoritative-window [1m] predicate.
1: docs target corrected to reference/configuration/providers.md + locales
2: exact seed before/after array, defaultModel unchanged, nano/sol excluded
3: evidence table moved to 000 (LEXICO-SPLIT-01)
4: per-model pi.dev provenance + two-leg selection rule (sol/nano out)
…ve seed policy

1: gpt-5.6-sol meets the two-leg rule identically to luna/terra (#748 field
report + pi.dev Responses declaration + JetBrains conditional chat failure);
excluding it was an inconsistent rule application. nano stays out (no field
report leg).
2: seed policy made explicitly additive (no removals); gpt-5-mini added as a
verified chat model so the chat regression fixture is honest; scenario 4
corrected.
…ecedence fixture

claim ledger and selection reference now say seven built-in / sol verified;
scenario 3 uses gpt-5.4-nano or gpt-5-mini for the opt-in direction since
sol is itself a default and cannot prove override precedence.
github-copilot fronts a mixed-wire catalog: gpt-5.3-codex, gpt-5.4,
gpt-5.4-mini, gpt-5.5, gpt-5.6-luna, gpt-5.6-sol, gpt-5.6-terra reject
/chat/completions for real Codex-agent traffic (function tools +
reasoning). Declare them as registry modelWireDefaults so every inbound
wire rides Responses while the provider-wide adapter stays openai-chat
for the chat-served catalog. Explicit modelAdapters still win in both
directions; gpt-5.4-nano stays out (no field report) as a documented
override example.

Evidence: issue #748 field runs, pi.dev wire declarations,
litellm#23332 (gpt-5.4), JetBrains LLM-29711 (gpt-5.6-sol).
Tests: tests/github-copilot-wire-defaults.test.ts — 22 cases incl.
captured-upstream-URL replay proof on all three inbound wires.

Consolidates the routing half of PR #746 (its sampling/credential-replay
half remains a separate parity/security unit). Closes #748.
… bounds

Three explorer passes found wave-1 landings already on dev (77243d9
framework, d1408b9 continuation cap+spill, 034d320 cache caps,
a616078 translator budgets), so the campaign narrows to refinements:
#841 admission boundary (direct-spill oversized, bounded snapshot read,
bounded replay), #847 collector per-call scope + mandatory budget + 502
normalization, #844 incremental frames + typed partial-EOF, #845 NOOP
(superseded), #843 fixed-size SHA-256 key identities, #840 ACL
timeout-memo release + destination keying.
All 7 blockers accepted: blob-ID key channel unbounded (045 becomes a
real fix), antigravity caps exclude key bytes, NUL-separator collision
ambiguity (length-prefix instead), cursor backlog/parser-cursor state
machine + frameWork-deferred EOF, collector test math + dual bridge
budget sites + exact 413 sites, ACL temp-only release API, state
typed-observability seams + failure/replacement scenarios.
001 synced with amended designs; 020 same-ID replacement split 8a/8b
with pruning qualification; 030 scenario-7 asserts lease release after
full serialized owner charge; 040 explicit rawBacklog byte cap incl.
headers + drain-to-quiescence EOF; 045 validate/digest before hex
expansion with separate key-bytes counter (64 MiB payload cap intact);
050 incremental length-prefixed hashing + replay_signature_oversized
skip-replay overflow contract.
…l replay

Oversized candidates (> 64 MiB resident cap) now write directly to
durable spill and install only the stub — they never become resident and
never demote unrelated chains; candidates above the new 256 MiB
single-spill payload ceiling are tombstoned at admission instead of
being retained unreadably. Snapshot loading refuses to parse files over
32 MiB and applies the same admission boundary to oversized rows.
Spill replay refuses payloads above the ceiling before read/parse with a
typed too_large reason (surfaced as spill_too_large; wire shape
unchanged). Snapshot selection now measures UTF-8 bytes, not UTF-16
length. Refines #841 on top of the wave-1 spill design.
…s escape hatch

reference/configuration/providers.md (EN + ko/ja/zh-cn/ru): the modelAdapters
row now names the built-in Copilot Responses-only defaults and the opt-in
path for models without one (gpt-5.4-nano example). guides/providers.md
gains the routing-precedence note (pin > modelAdapters > registry default
> provider adapter).
Envelope truth: the payload ceiling is enforced against the real
{version,...} spill envelope after publication (a candidate within the
wrapper's size of the cap was previously retained unreadably). Snapshot
size gate uses statSync so symlinked oversized snapshots cannot bypass
it. Over-ceiling same-ID tombstones now defer the old generation's
unlink until the tombstone is durable (same crash rule as
replaceSpillEntryAtomically, whose catch path gets the same fix).
directSpills only counts actual stub installs. Tests: exact-envelope
boundary, no-read proof (too_large beats missing), deferred-unlink
ordering for tombstone and spill replacement, symlinked snapshot
refusal, tighter same-ID accounting bound.
…ntics

adopts #860's reviewed fail-closed semantics (undefined strips; explicit
config is the escape hatch); names preserveResponsesReasoningContent as the
reasoning-replay flag following the statelessResponses flow; maps every
edit point on the current tree.
A snapshot symlinked to a FIFO or device (isFile=false, size=0, e.g.
/dev/zero) fell through to readFileSync and could block or stream
unbounded input; only regular files within the size ceiling are parsed
now.
1: research survey moved to 000 (LEXICO-SPLIT-01)
2: guides/codex-app-models.md x5 locales added to docs file map
3: options.serviceTier clearing required + tested
4: ocxr1-strip-under-preservation negative case added
5: comment wording calibrated (accepted replay = verified; required = inference)
…to 502

The non-stream Chat collector charged streamed tool arguments to a
generic retained_collectors scope, so one call could consume nearly the
whole 32 MiB turn budget instead of the 2 MiB per-call limit; arguments
now open a per-call scope (kind tool_args, wire-index key) and close it
only after the final serialized owner is charged, with scopes released
on every error path. Provider-controlled overflow in the collector is
now 502 upstream_error on all three mapping sites (was 413
invalid_request_error on two), matching the adapter/bridge contract.
buildResponseJSON/bridgeToResponsesSSE no longer have an unbounded
no-budget path: omission creates a default turn budget (disposed with
the call/stream) instead of skipping accounting.
…epSeek reasoning replay

Two DeepSeek wire fixes, one capability flow:

1. service_tier is an OpenAI-only Responses parameter, but fast mode
   injected it for every Responses provider. A provider-level
   supportsServiceTier capability now gates it after the final route is
   settled: canonical openai/openai-apikey keep fast-mode inject/remove
   (unset fast mode preserves a caller value); deepseek and
   volcengine-agent-plan strip it; unclassified providers fail closed
   unless explicitly opted in. Stripping also clears
   options.serviceTier so logging never mislabels a removed tier.
   Adopts PR #860's reviewed fail-closed semantics.

2. sanitizeReasoningInputContent blanked reasoning content for EVERY
   Responses provider — a rule only the ChatGPT native backend needs.
   DeepSeek's Responses API accepts plaintext reasoning replay, so
   providers flagged preserveResponsesReasoningContent keep it
   (ocxr1 envelopes are still stripped). Fixes the local half of #875:
   continuations after tool calls no longer reach DeepSeek with emptied
   reasoning items.

Both fields flow registry -> providerConfigSeed -> enrichProviderFromRegistry
-> router backfill without overriding explicit config.
Tests: tests/service-tier-capability.test.ts + tests/deepseek-reasoning-replay.test.ts
(18 cases incl. live handleResponses payload capture).
…ent reference rows, capability-gated fast-tier guides

reference/configuration/providers.md (EN/ko/ja/zh-cn/ru): rows for the two
new provider fields. guides/codex-app-models.md (all five locales): the
blanket 'routed non-OpenAI models strip service-tier metadata' wording is
now the capability-gated fail-closed behavior with the explicit opt-in —
closing #860's open docs review issue.
Final owner transfer is wrapped: a copy-charge overflow now releases the
copies already charged, closes every open call scope, and surfaces the
same typed 502 upstream_error instead of a raw budget exception. The
translation_buffer_limit classification is 502 upstream_error on the
remaining streaming sites (fail() override, response.failed handler,
processing catch) and the defensive JSON replay path, so streaming,
collector, and non-stream clients agree; genuine client-request 413s
are untouched. Owned default SSE budgets are disposed at every
stream-death path (terminal close, incomplete terminal, torn-down
controller, heartbeat failure, cancel) after the final charges, never
inside reportTerminal. New test seam translatorLiveBudgetCountForTests
proves disposal; collector tests now cover final-copy overflow cleanup
and exact surviving charge.
A client cancel during a pending upstream next() disposed the owned
budget while the in-flight step still processed the late event,
reanimating charges outside liveBudgets; step() now bails immediately
after the await when closed/cancelled (emit() was already guarded).
The defensive JSON replay status ternary for translation_buffer_limit
is 502 (was 413) — every path now agrees. The final-copy overflow test
threshold is corrected so the overflow actually activates in the final
owner transfer (250, not 150). New seams:
translatorAggregateCurrentBytesForTests; cancel-during-pending-next
regression asserts live count and aggregate bytes return to baseline;
endpoint regression pins the replay 502/upstream_error shape.
Seeding them via providerConfigSeed broke the management API's canonical
openai seed comparison (exact key set) — 4 management-provider-validation
failures. Follow the modelWireDefaults philosophy instead: the registry
holds the defaults, providerConfigSeed stays free of them so an explicit
user value stays distinguishable, and enrichProviderFromRegistry +
the router backfill supply them from the entry directly.
Breaking to upstreamDone still ran the EOF path, whose
closeCurrentMessage()/retainFinishedItem() charges the disposed budget
directly (the emit() closed-guard does not cover retention); a
cancelled step now returns immediately like the gated early-return
path. The regression now synchronizes on the second upstream next()
having actually started before cancelling.
…ts-inject + generator override

desktop-profile and model-info guards already landed; the open halves are
the registry map, #854's withSubagentContextMarker port, and the stale
jawcode generator override contradicting its own committed output.
… EOF

The live transport concatenated every incoming chunk with the entire
pending remainder (O(n^2) on a large incomplete frame) and settled
stream end immediately: complete frames plus a trailing partial settled
as silent success. The backlog is now a {buf,start,end} cursor with
amortized growth (per-chunk O(chunk)) and lazy prefix compaction; a new
framing export consumeConnectFrames reports consumed raw bytes without
materializing a remainder copy. Accounting moves to raw used bytes
(headers included), closing the tiny-frame/header-flood gap in the
payload-only counter. Stream end now drains frameWork to quiescence,
fails typed frame_incomplete on any unconsumed remainder (unless the
close was expected), and keeps the zero-frame unexpected-EOF
classification.
…ts + full regression set

my stale check misread the bedrock rows: the anthropic section of the
generated metadata still pins sonnet-4-6 at 200k with a catalog test
blessing it; file map now includes the generated rows, codex-catalog
test, registry-parity assertions, and #854's complete test set
(modified 372k test + five additions incl incomplete-metadata branch).
…tative [1m] in generated profiles

Two halves, consolidated from PRs #839 and #854:

1. ANTHROPIC_MODEL_CONTEXT_WINDOWS omitted claude-opus-4-7, claude-opus-4-6,
   and claude-sonnet-4-6 although all three ship in ANTHROPIC_MODELS —
   they advertised max_input_tokens null, emitted no [1m] picker row, and
   Claude Code accounted them at its 200k default. All three are
   documented at 1M by Anthropic (Opus 4.6 2026-02-05, Opus 4.7
   2026-04-16, Sonnet 4.6 2026-02-17). The generated jawcode metadata and
   its 200k-pinned catalog test move with the registry; the stale
   CONTEXT_WINDOW_OVERRIDES generator pin is removed.

2. Generated subagent defs marked [1m] with the MAIN-SESSION auto-context
   predicate, so a 372K route was written [1m] into generated profiles —
   accounted at 1M with no compaction pairing in the subagent. Generated
   defs now mark only authoritative >=1M windows, strip inherited unsafe
   markers to bare, preserve genuine routed [1m] ids (kimi/k3[1m]) and
   provider caps, and keep selectors with unknown windows as-was.
   Main-session env-slot marking is unchanged.

Tests: 372k roster test corrected + five new regressions (catalog-derived
1M markers, routed [1m] preservation, 350K cap unmarking, marker-case
precedence, incomplete-metadata preservation), catalog 1M contract,
registry parity assertions for all three windows.
consumeConnectFrames now returns payload VIEWS into the backlog (safe:
append-only at end, compaction/growth replace the buffer), so the charge
transfers to the frame lifecycle instead of reserving a second copy —
an exact 16 MiB effective payload peaks at 16 MiB + 5 and completes,
matching the documented boundary. One idempotent releaseBacklogLease
owns the raw remainder on every terminal path (failAndClear, EOF
classifications, first-frame timeout); the data-handler catch no longer
releases the chunk charge after a successful append, which understated
the retained backlog. Tests: exact-boundary live probe, lease-to-zero
after frame_incomplete, 1/3/7/64 KiB chunk sweep (RED on both counts
before this repair).
When reserveTransportBytes rejected an over-cap chunk, the data-handler
catch released the chunk size anyway, debiting unrelated live ownership
(clamps masked the imbalance). The catch now releases only when the
reservation succeeded but the append never ran; after an append the
terminal backlog cleanup owns the bytes. Regression: two transports on
one shared budget — A parks an incomplete frame, B's own incomplete
frame overflows; A's lease is byte-exact throughout and the budget
returns to zero.
kimrinking-cell and others added 7 commits August 3, 2026 03:47
…d-scan repair, strict EOF predicates)

Review findings on the stacked contributor fixes, all red-green verified:

- core.ts upstream JSON branch read the whole body with an unbounded .text();
  every non-streaming upstream — now including WebSocket turns deliberately
  answered with bounded JSON — could grow proxy memory without limit. The read
  goes through relay.readBoundedResponseText (32 MiB ceiling, body cancelled on
  overflow) and fails closed with a 502 instead of parsing a partial body.
- anthropic lastValidJsonObject collected every brace offset into two arrays
  before its candidate cap, so brace-dense hostile input cost O(n) index storage.
  It now scans backwards from the end with lastIndexOf and never materializes an
  index; inputs above 1 MiB are not repaired at all.
- an empty text_delta marked sawVisibleText, letting a cut-off tolerant stream
  complete as a successful empty answer; only non-empty text authorizes tolerant
  completion now.
- a translator-budget overflow could be followed by tool_call_end/done when the
  generator was fully drained; the budget error now returns immediately as the
  single terminal event.
… UTF-8 cap)

Re-audit findings on 9794e24:

- readBoundedResponseText awaited reader.cancel(), so a broken stream whose
  cancellation never settles would hang the overflow path instead of returning
  the documented 502. The custom helper is replaced by the house primitive
  readBoundedResponseBody, which cancels fire-and-forget with synchronous-throw
  protection and adds total (180s) and inactivity (30s) transfer deadlines on
  top of the byte ceiling; oversize and stalls both fail closed, and a partial
  body is never parsed.
- bounded-body.ts gains a maxBytes option (default unchanged at 64 KiB) and
  accumulates into a geometrically growing single buffer, so per-chunk metadata
  cannot amplify beyond the payload budget on large ceilings.
- the repair byte cap measured UTF-16 code units; astral text could enter the
  parse attempts at 4x the intended bytes. utf8BytesExceed measures the exact
  UTF-8 length with early exit and no allocation; regression test covers a
  600k-code-unit, 1.2 MB input that a length check would have admitted.
…in the repair cap

The 4-byte branch of utf8BytesExceed skipped the next code unit whenever it
existed, without checking it is a low surrogate — so a run of high surrogates
was counted at 2 bytes per unit while TextEncoder emits 3. The pair path now
requires the next unit in 0xDC00..0xDFFF; anything else counts 3 bytes without
skipping. Regression: 200k high-surrogate pairs (400k code units, 1.2 MB on
the wire) decline repair instead of being admitted at 800k counted bytes.
Each GET /api/system/memory read costs ~600 ms on the shared ubuntu runner
(the route samples the live process), and the test performs eight reads —
5.2 s against bun's 5 s default timeout, which is how it flaked red on the
wp6 head while passing locally and on macos. Bump the per-test timeout to
20 s; the assertions and read count are unchanged.
…weep test

The symlinked-directory sweep test drives the real load path, whose stale-temp
sweep checks liveness with kill(pid, 0). The hardcoded dead pid (4242/4243)
collided with a live process on the macos CI runner, so the temp survived and
the assertion flaked red while the same commit passed locally and on ubuntu.
Probe upward for a pid that returns ESRCH instead; the injected-liveness tests
are untouched.
…nners

Both execute real SQLite writes and filesystem moves, the same intrinsic-cost
class as the five neighbors that already carry STORE_BUDGET_MS — they simply
never got the budget, and flaked at ~5.6s against bun's 5s default on the
windows-latest runner. Ablation check: both assert real quarantine/permanent
behavior (locally green in <0.5s), so the budget is headroom, not cover.
…ion test

The live PowerShell enumeration test already tolerated one transient empty
result, but on a sufficiently contended windows-latest runner execFileSync's
8s production deadline fires instead and the error propagates by design —
that throw was not tolerated, flaking the job. Catch it the same way and add
a third attempt with a longer settle; production behavior is unchanged.
@lidge-jun
lidge-jun merged commit 075c2f3 into dev Aug 3, 2026
14 checks passed
lidge-jun added a commit that referenced this pull request Aug 3, 2026
… mechanically

go/internal/cli/config_parity.go was committed by a broad `git add` three times
during the #820 campaign. Two of those were caught and reverted (2101d50,
58e8718); the third rode a86ee03 into the #892 merge, so 682 lines of the
retired Go experiment are now tracked on dev — the one and only tracked file
under go/, absent from main, preview, and v2.10.0.

Nothing in src/, the build, the typecheck, or the test path reads from go/, so
this is dead weight in every clone rather than a functional regression. The
commit that introduced it says "untrack ... again" in its own message, which is
how it passed review: the intent was right and the index was not.

.gitignore alone cannot prevent the repeat, because an already-tracked path
stops honoring the ignore rule — so the guard lives in tests/repo-hygiene.test.ts
next to the .codexclaw/gitlink invariants, driven red once by re-adding the file.
lidge-jun added a commit that referenced this pull request Aug 3, 2026
fix(security): close the agent star-consent bypass found auditing the #892 merge
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants